Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NEW][ENTERPRISE] Push Notification Data Privacy #18254

Merged
merged 23 commits into from
Jul 21, 2020

Conversation

pierre-lehnen-rc
Copy link
Contributor

Proposed changes

This PR adds a new Enterprise setting to enable push notifications to send only the message ID instead of the whole message. It also adds a new API endpoint to retrieve the full data using a messageId as argument.

Issue(s)

How to test or reproduce

Screenshots

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Hotfix (a major bugfix that has to be merged asap)
  • Documentation Update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Changelog

Further comments

@pierre-lehnen-rc pierre-lehnen-rc added this to the 3.5.0 milestone Jul 13, 2020
@lgtm-com
Copy link

lgtm-com bot commented Jul 13, 2020

This pull request introduces 1 alert when merging 79bc9c5 into c492c61 - view on LGTM.com

new alerts:

  • 1 for Property access on null or undefined

@lorek123
Copy link

Could you consider moving this to free version? I would like to use this, but for my usecase using enterprise is not an option.

app/settings/server/functions/settings.ts Outdated Show resolved Hide resolved
server/publications/settings/index.js Outdated Show resolved Hide resolved
ee/app/license/server/license.ts Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Jul 14, 2020

This pull request introduces 1 alert when merging 00a48a2 into 6429353 - view on LGTM.com

new alerts:

  • 1 for Property access on null or undefined

ee/app/license/server/license.ts Outdated Show resolved Hide resolved
ee/app/settings/server/settings.ts Outdated Show resolved Hide resolved
app/api/server/v1/settings.js Outdated Show resolved Hide resolved
Copy link
Member

@sampaiodiego sampaiodiego left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

push-privacy module should be added to enterprise bundle at

app/push-notifications/server/lib/PushNotification.js Outdated Show resolved Hide resolved
app/push-notifications/server/lib/PushNotification.js Outdated Show resolved Hide resolved
app/api/server/v1/push.js Outdated Show resolved Hide resolved
app/lib/server/startup/settings.js Outdated Show resolved Hide resolved
@sampaiodiego sampaiodiego changed the title [NEW] Push Notification Data Privacy [NEW][ENTERPRISE] Push Notification Data Privacy Jul 21, 2020
@sampaiodiego sampaiodiego merged commit c4f2dd8 into develop Jul 21, 2020
@sampaiodiego sampaiodiego deleted the enterprise-settings branch July 21, 2020 03:38
@paulkernstock
Copy link

Too bad this is enterprise only! Shouldn't privacy features be made available to everyone using Rocket.Chat?

On my instance I have disabled sending information about sender and message content over push and highly anticipated this feature.

Those users who were invited to Rocket.Chat with these push privacy setting disabled don't even know that their name and message are sent in plain text through the servers of Apple and Google. This is a major concern in my opinion. (maybe q.v. #9027 for more information)

I am continuously donating to RC, although for private use enterprise or pro for me is not an option.

@engelgabriel
Copy link
Member

@paulkernstock we hear your concerns and I hope you do understand that we constantly strive to strike a balance between what we provide for free to our community and what we offer on the commercial product to make it attractive. It is never an easy decision and our desire is always to give as much as possible for the community. This specific feature has been a strong argument for the sales team to close deals, which in turn allows us to keep developing the product further. Having said that, I also understand that not everybody has the budget to move to the fill enterprise license while the only need some specific paid features, so we are going to provide features like this one via individuals app on our marketplace for small deployments, so donors like you will be able to buy those features for about the value of your donations. I believe this will be a win-win situation, so you can continue to support the project and we can say thank you with some premium features in return.

@paulkernstock
Copy link

@engelgabriel Thanks for your reply and your reasoning. Of course is this feature required by companies that are also in the position of affording a paid plan for their teams. Thus the reasoning from your own sales team. But I'll have to insist that privacy is one of the major concerns for most community users and a motivation to prefer on-premise RC over Slack, Telegram, Skype, WhatsApp, MS Teams or what have you.

In the course of the discussion about RC demanding server registration and the announcement of future push limitations, it only now came to some that this privacy "breach" exists (q.v. https://forums.rocket.chat/t/enforcing-registration-requirement-to-utilize-push-gateway/7545)!

I generally think it's a common misconception, that OSS has to be free of charge. It's partly crazy good software out there like RC or Nextcloud that people can use freely, but someone has to pay those who make it, right? But on the other hand this software gains a lot of its value from their idealistic community who in turn contribute (argumentation, feedback or code). Keeping such fundamental features or improvements from this part of the community really sends out a false and disturbing signal. There are loads of other functionality and caps that can easily be reserved for paying customers.

The marketplace solution you mentioned is an interesting approach that I will definitely give a try (once the registration of my server does not fail anymore). But this means should be reserved for the non-fundamental features as I pointed out. As a suggestion: Once your back-end team has found a reasonable limit for free push notification that does not exclude the majority of community users out there, sell a push notifications capacity increase on the marketplace.

But let this be my urgent plea, don't sell privacy!

@reetp
Copy link

reetp commented Jul 22, 2020

I think that as more features are only added to Enterprise, Rocket.Chat really should think about changing the sales and marketing which is really false advertising.

"Unlimited & Open-source​"

With separate Enterprise features, 'Unlimited' is not strictly true.

"Open Source but Limited Options unless you pay for Enterprise" is a more accurate strap line, or "Unlimited features in Enterprise version" (Which is fine. Just be honest about it)

This is a simple privacy feature which should really benefit all users. I'm not sure it is a clincher for sales deals.

I would imagine it may have some potential impact with respect to possible data privacy laws. How does it work with say your claims on say GDPR compliance?

Community compliance
The above will allow our community members to build and deploy GDPR compliant systems and services.

I'm not sure Rocket can claim to be compliant if this sort of data is flying about un-encrypted in the ether...... (I had no idea, and it makes me extremely uncomfortable at the thought)

And yes, this is still open and should be addressed.

#11142

@TheReal1604
Copy link
Contributor

^ As others above said. I deactivated the sending of message contents on all my deployments there we are using the push gateway. Paywalling an essential privacy feature behind a "Enterprise Edition" is a no-go.

I can understand that you need reasons for your Enterprise Edition, but this is a move in the wrong direction, as your entire user base, who probably doesn't have the ability to provide a custom app for their deployment, would benefit from it. Not just large organisations which exploit the OSS model, which try to get the most out of it for free.

@sampaiodiego sampaiodiego mentioned this pull request Jul 28, 2020
@dbenz04
Copy link

dbenz04 commented Jul 28, 2020

If we are on the free version and have the "Show Message in Notification" setting turned off, are the full notification message contents still being sent to Apple and Google?

@rodrigok
Copy link
Member

If we are on the free version and have the "Show Message in Notification" setting turned off, are the full notification message contents still being sent to Apple and Google?

No

@la-coste
Copy link

la-coste commented Aug 4, 2020

I assume push notifications aren't GDPR compliant, unless you pay for Enterprise? This makes it basically illegal to use in the European Union. Guess we will have to move our communications to another platform.

The lack of transparency in regards to what is encrypted, what is not, what is transferred to other servers and what is not, what is transferred outside of the EU and what is not, etc, is really frightening. What data is "shared" with Rocket.Chat Technologies Corp.?

@rcadmin19
Copy link

In Community Edition server version 3.5.1 there is still "Show Message in Notification" and "Show channel/group/username in Notification" options and those can be disabled. Is there a plan to hide those in CE version or what is the point of this "new" enterprise feature only? This is pretty confusing IMO.

@rodrigok
Copy link
Member

@rcadmin19 Show Message in Notification and Show channel/group/username in Notification prevents the server to send that information to the mobile apps. The Enterprise Privacy options don't send that information as well but sends the message's id telling the mobile apps to request the rest of the date from the server directly.

So in CE you can still block the information to be transmitted, but only on EE the mobile apps can request the information from the server before showing the notification.

@rcadmin19
Copy link

Rodrigok, thank you for the information. Unfortunately there is a bug which I reported several months ago: #17135. We analyzed this again in version 3.5.2 and noticed that "Show channel/group/username in Notification" button does nothing. This can be verified also from source code. So this is a security issue: Username, users name and room name is sent anyway despite of that button position!

@bleomycin
Copy link

@engelgabriel As a home user as well who has 0 problems paying a fair price for software I use and enjoy will there be an announcement somewhere if/when this features is available as an app purchase that you mentioned? I often have trouble finding news of features and the official documentation is often outdated making it difficult for someone who only checks in occasionally to know what is going on.

@jliebers
Copy link

jliebers commented Jul 29, 2022

Sorry for asking two years later, but is there any update or workaround existent so that we can have privacy in push notifications without leaking them to the gateway owner(s)? E. g., the ability to only buy this feature only or something in this direction?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.